Migrating cuda-python samples from cuda-samples - #2266
Conversation
mdboom
left a comment
There was a problem hiding this comment.
Is the plan here to migrate the samples here from cuda-samples, and this would be the new canonical location? I ask only because the check_spdx.py change suggests it's just "syncing" them.
We already have examples in cuda_bindings/examples and cuda_core/examples. I think it would be better to add these in one of those two places (depending on whether the sample uses cuda_core or not) rather than creating a third place. There is already infrastructure there to run those as part of the main test suite. (I also think it's preferable to test these as part of the cuda_bindings and cuda_core test suites rather
There may or may not be redundancy between some of these examples and the ones already in this repo. I'm haven't verified that, but maybe having an agent search for any that are explaining redundant concepts might pare this down a bit.
We also put a bunch of work into standardizing the examples in this issue. You might be able to point your agent at that issue and get it to perform some cleanup on these.
The other significant difference is that these examples all have extensive README.md files. The existing ones here don't. Personally, I think the README.md content could go in the top of each example file as a docstring, but I'm open to other ideas.
|
Sorry i forgot to add more context. Leo had it from a meeting we had a while back with the samples team but not everyone was there. Yes, the idea is to move the source of truth of the cuda-samples to cuda-python. So we can have CI and so on here. We agreed on syncing the samples to the cuda-samples repo on each release so that they are also part of the broader existing samples. That's another process that would be independent of what we have here and the samples team will do that. I did not removed any existing examples yet but we should migrate those to the standard we have in cuda-samples, basically have the markdown and comments in the code explaining. I can do that. I am mostly looking for feedback on the CI parts, because we need the samples to be standalone (for when we sync) we have that pytest wrapper. |
|
I have no objection to moving these into their own top-level organization. However, they should be tested as part of the main The existing examples are also included in the docs and we should figure out how to address that so there is one canonical online place where people would find things. (But that could be follow-on work, possibly managed by the samples team, once the merging of the 3 sets of examples happens) |
|
Thanks for driving this, Daniel, and for early feedback, Mike!
+1 as my high-level feedback before calling it a day/week. |
|
The top level Adding it as part of the tests of |
Resolves conflicts caused by concurrent changes on main: - Relicense (#2293) updated SPDX headers on cuda_bindings/examples/*.py and cuda_bindings/cuda/bindings/_example_helpers/*. All of those files were removed on this branch, so main's edits are discarded via git rm. - Bindings-side test_examples.py, cuda_core/examples/pytorch_example.py and cuda_core/tests/example_tests/test_basic_examples.py were removed on this branch; main's edits to them are discarded. - scripts/run_tests.sh was removed on main (#2271); accept the deletion. - ci/tools/run-tests: keep this branch's optional sample-deps install (nvtx, pillow) but pick up main's rename CUDA_VER_MINOR -> TEST_CUDA_MAJOR_MINOR. - Four new texture / GL-interop examples added to cuda_core/examples/ on main (gl_interop_fluid.py, gl_interop_fluid_numba_cuda_mlir.py, gl_interop_mipmap_lod.py, texture_sample.py) are migrated into their own directories under samples/ with README.md and requirements.txt. Added a DISPLAY guard to glInteropFluid and glInteropMipmapLod so they waive with exit code 2 on headless runners, matching glInteropPlasma. - ruff.toml: ignore RUF059 under samples/** (matches the pre-existing ignore for examples/**). - .spdx-ignore: drop the obsolete cuda_bindings/examples/* line. Verified end-to-end: - pixi install -e samples: solves cleanly. - pytest cuda_core/tests/example_tests/test_samples.py: 46 passed, 9 skipped (all legitimate: no DISPLAY, no P2P, sub-Hopper GPU, or intentionally omitted deps), 0 failed. 55 samples collected. - ruff check + format: clean. - toolshed/check_spdx.py: clean on edited files.
Code reviewFound 2 issues:
cuda-python/cuda_core/tests/example_tests/test_samples.py Lines 30 to 38 in 035cf32
cuda-python/samples/cuda_core/stridedMemoryViewCpu/stridedMemoryViewCpu.py Lines 116 to 123 in 035cf32 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
rparolin
left a comment
There was a problem hiding this comment.
The cuda_bindings samples are numbered while the cuda_core samples are not. I don't know if that's intentional or not. But otherwise lgtm.
|
Yes, that was intentional because its how we have them at the moment. I personally don't like the enumeration but I can go either way. Also addressed the claude review. |
|
/ok to test f5945e8 |
|
/ok to test 3af1797 |
Relative imports fail when example_tests has no __init__.py; import run_samples as a top-level module instead.
|
/ok to test 053b2cf |
- systemInfo: guard get_process_name() call behind CUDA_BINDINGS_NVML_IS_COMPATIBLE to avoid NameError when NVML bindings are incompatible with the driver. - vectorAdd: replace cp.random.rand() with NumPy-generated data transferred via cp.asarray() to avoid a dependency on libcurand which is not present in the cuda-13x wheel environment.
|
/ok to test 4906537 |
CuPy's FFT backend requires libcufft (so.11 or so.12), which is not bundled in the cuda-13x wheel environment used in CI. Probe cupy.cuda.cufft at startup and exit with the runner waiver code (CUDA_PYTHON_SAMPLE_WAIVER_EXIT_CODE=77) so the test is reported as SKIPPED rather than FAILED.
…tSignalAnalysis CuPy's FFT backend requires libcufft (shipped by nvidia-cufft-cu12 on PyPI). Adding it to the PEP 723 inline script block and requirements.txt lets the sample runner automatically waive the test when cuFFT is not installed, rather than crashing with an ImportError at runtime.
…umpyVsCupy cp.dot() requires libcublasLt (shipped by nvidia-cublas-cu12 on PyPI). Adding it to the PEP 723 inline script block and requirements.txt lets the sample runner automatically waive the test when cuBLAS is not installed.
…gram cp.random requires libcurand which is not installed in the cuda-13x wheel CI environment. The random data is only used as histogram input, so generating it on CPU via numpy and uploading with cp.asarray() is equivalent and removes the curand dependency.
|
/ok to test 014c86b |
… is unsupported ManagedMemoryResource with preferred_location requires concurrent managed memory access support, which is not available on all devices (e.g. RTX 4090 in the CI environment). Catch CUDA_ERROR_NOT_SUPPORTED at runtime and exit with the runner waiver code so the test is reported as SKIPPED rather than FAILED. This mirrors the existing Windows waiver already in the sample.
…cess is unavailable Per CUDA docs (cuMemPoolCreate): managed memory pools require all devices to have non-zero concurrentManagedAccess; otherwise CUDA_ERROR_NOT_SUPPORTED is returned. This attribute is 0 on Windows, WSL, and some Linux VM environments. Check device.properties.concurrent_managed_access before attempting to create the ManagedMemoryResource and exit with the runner waiver code when it is False, mirroring the existing Windows waiver already in the sample.
…cess is unavailable Managed memory pools (cuMemPoolCreate) require all devices to have non-zero concurrentManagedAccess; otherwise CUDA_ERROR_NOT_SUPPORTED is returned. This attribute is 0 on Windows, WSL, and some Linux VM environments. Move the concurrent_managed_access check to just after device info is printed and simplify the message, keeping it alongside the existing Windows waiver.
NVIDIA WSL User Guide states that only legacy CUDA IPC APIs are supported from driver R510; the newer IPC memory pool API (cuMemPoolCreate with POSIX FD handles) is not supported on WSL2. Detect WSL by checking for 'microsoft' in /proc/sys/kernel/osrelease and return False from check_ipc_support() so the sample exits cleanly with a waiver instead of crashing with CUDA_ERROR_INVALID_VALUE.
…rrent managed access is unavailable Both samples use ManagedMemoryResource which requires concurrent managed memory access (concurrentManagedAccess != 0). On WSL and Windows this attribute is 0, causing CUDA_ERROR_NOT_SUPPORTED at pool creation. Add concurrent_managed_access check after device init, consistent with the existing Windows waiver and the fix applied to blurImageUnifiedMemory.
…UDA context nvmlSystemGetProcessName returns NVML_ERROR_NOT_FOUND when the process has not yet established a CUDA context (i.e. no cudaMalloc or equivalent has been called). This is expected NVML behavior per the API docs. Guard the get_process_name() call with a try/except NotFoundError so the sample gracefully prints N/A instead of crashing.
|
/ok to test 014c86b |
|
/ok to test decb50e |
Description
Moving the cuda-python samples from the cuda-samples repo.
Migrating also the sample runners and added a small pytest wrapper to run the test.
Let me know if we want to do any changes to when these are run.
closes
Checklist